Add video recording to NixOS VM tests#41165
Conversation
|
Success on x86_64-linux (full log) Attempted: qemu_test Partial log (click to expand)
|
|
Failure on x86_64-darwin (full log) Attempted: qemu_test Partial log (click to expand)
|
|
Related: #33299 |
|
Failure on aarch64-linux (full log) Attempted: qemu_test Partial log (click to expand)
|
|
@dotlambda: This is basically a follow-up to that PR and what I suggested back then. Having a custom UI module in QEMU also has the advantage that we can watch regions for changes, which is particularly useful for making tests with keyboard/mouse input more reliable. Right now we handle input blindly based for example on the assumption that a particular application has started, but we don't actually know whether an input field has focus or even the application is accepting input (we only check whether there is an X window, which could be empty) at all. With access to frame deltas we can simply watch for a certain region to change, press a key, wait for another change (and if it doesn't change, repeat the keypress), press the next key and so on. |
|
There is a however still an issue with the videos, because not all frames are written to the intermediate file. The amount of frames received from QEMU is correct but it seems that the UI module doesn't seem to get all the frame updates from the console. I'm adding a WIP label until I've resolved that. |
|
Needed to fixup the first commit because this would break QEMU on non- |
|
@GrahamcOfBorg build qemu_test |
|
Failure on x86_64-darwin (full log) Attempted: qemu_test Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: qemu_test Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: qemu_test Partial log (click to expand)
|
|
@GrahamcOfBorg build qemu_test |
|
Success on x86_64-linux (full log) Attempted: qemu_test Partial log (click to expand)
|
|
Failure on x86_64-darwin (full log) Attempted: qemu_test Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: qemu_test Partial log (click to expand)
|
There was a problem hiding this comment.
IMHO the Nixpkgs repo is really not the place to store large C source files.
There was a problem hiding this comment.
I could also put them into a separate repository if you'd prefer that.
|
While this is cool, it significantly increases the complexity of our testing infrastructure, in particular adding a custom version of QEMU that might be a PITA to maintain. How large are the video files generated by this for a NixOS release? Our storage costs are already pretty high... |
|
@edolstra: The intermediate video files are up to 2 MB (they only contain compressed frame deltas, although lossless) and the encoded videos in WebM format are around 2-5 MB. If that's too much we could alternatively just not encode them and only make the intermediate format available so that videos can be encoded on a per-case basis. |
This is mainly for Hydra so that the videos show up in the build products. Encoding this to WebM takes a while especially for long test runs, but it also helps debugging tests without the need to manually run the encoding process on the actual output path of the test runner's videos. Signed-off-by: aszlig <aszlig@nix.build>
When writing with header_size - 1 we shouldn't check whether header_size has been written. I also added an exit(1) to make sure this is fatal, because otherwise things such as the issue here go completely unnoticed. Signed-off-by: aszlig <aszlig@nix.build>
For some tests such as runInMachine, there is neither a .videos attribute nor does it actually need to record videos. However, right now it still does record videos so we need to fix that soon. Signed-off-by: aszlig <aszlig@nix.build>
This is actually quite common whenever video frames are written that a signal can interrupt a call to write(). The first option that came into my mind was to use sigprocmask() to make sure that we don't get interrupted while writing the frame data. However this would also introduce some overhead. So instead, we're now just ignoring a frame update/switch whenever we reach the end of the file, so we don't have that overhead and also only loose one frame to the end of the video stream because we do actually flush the gzip buffer in the atexit() handler. Signed-off-by: aszlig <aszlig@nix.build>
When building with Nix there is a NIX_BUILD_CORES environment variable set to either the default, which should be equal to av_cpu_count() or a user-set value, so let's respect that. If the value is 0 or unset (not inside a Nix build), we use av_cpu_count() instead. Signed-off-by: aszlig <aszlig@nix.build>
If we only want to run tests without actually encoding the videos, it doesn't really make sense to pull in the FFmpeg dependency. When debugging tests we can still either use the encoder manually or simply append the .videos attribute in order to get the encoded videos (which then of course will pull in the FFmpeg dependency). Signed-off-by: aszlig <aszlig@nix.build>
0d0f82e to
7566b3e
Compare
|
Rebased against current master. |
flokli
left a comment
There was a problem hiding this comment.
As much as I appreciate the work put into this, I still don't think patching qemu downstream is the right way forward. Quoting my original comment from three years ago:
Did you try approaching upstream qemu with the problems you're facing? I'm not sure if we do ourselves much of a favor shipping our own custom fork of qemu, and tooling to produce a somewhat ffmpeg-specific intermediate format etc. We certainly can't be the only ones facing this. I'd rather see this somewhere inside or alongside qemu, or spice being fixed to not drop frames (if the underlying architecture allows this).
Please approach upstream to get either this or another reliable way of video recording merged. I don't think this is suitable to ship in nixpkgs only.
I'd like to avoid putting such a module upstream since currently it's too Nix-specific and I also have plans for using this same module to add UI test functionality (eg. selecting/matching regions for change or constraining OCR). Furthermore, upstream already rejected a similar implementation (although that one used ffmpeg, while ours does not). |
|
Addendum: Since QEMU 7.0 there is a new D-Bus display, which as far as I can see calls a D-Bus listener for every frame update. This is a lot more close to what we want, but it seems to always write the full frame image even if only a subset has changed. It's not a show-stopper though since we can easily work around this limitation if we even need to. I'll experiment with that a bit to see whether it's a viable option or whether it has similar drawbacks such as SPICE/VNC. |
|
@Aleksanaa: What's the reason for the close? |
|
Sorry, was just sorting out things being inactive for a long time. |
|
I think it's fair to close inactive pull request if the author doesn't work on it because it still requires reviewers to revisit the pull request from time to time. |
No need to be sorry. I'm totally fine with this, I was just curious to know why. |
Sometimes, it's a bit difficult to debug VM tests post-mortem, especially when GUIs are involved. So this adds a new
nixos-testQEMU UI module toqemu_test, which writes raw video frame deltas to an intermediate file (the reasons for this are detailed in the commit message of db8e70f8636440f2c685461e20fc981170301981), so that it can be encoded to a video file at a later stage.Encoding of the video is done using a helper tool called
nixos-test-encode-video, which encodes the intermediate format into a more commonly recognized one determined by the filename given (for examplenixos-test-encode-video foo.video bar.webmwill encode the intermediatefoo.videointobar.webmusing a WebM container with VP9).NixOS VM test runners now have another attribute called
videos, which will gather all the build products from the normal test runner, encode the videos to WebM and add them to the build products. This could see a bit of improvement in Hydra so that those videos are directly displayed in the browser instead of a download.I've also added usage of the
.videoattribute inrelease.nix, so that this is done by default but doesn't increase test run time when building the tests directly without going throughrelease.nix.A Hydra jobset of this branch is available at https://headcounter.org/hydra/jobset/aszlig/nixos-tests.
Partial example GIF from the enlightenment test:
Converted back to draft since there are still a few issues to solve: